GLTFLoader: Refactor animation methods to be accessible from extensions on parser #26126
Conversation
|
@hybridherbst I have no concerns with this PR — Based on #24193 (comment) I can understand this might not be ideal from your perspective ... would you be OK with our merging this PR, and we can think about ways to reduce duplication in later PR(s)? |
|
Yes, would be happy with merging this PR as it would finally allow us to release KHRAnimationPointer support as separate file and it "just works" with three.js. The remaining duplication should be solved later when/if that is merged back in in one way or another. Might also be a candidate for testing a dynamic import of an entire extension implementation only when needed (only briefly touched on this with @mrdoob) |
|
This can be merged now, right? :) |
…ser (mrdoob#26126)" This reverts commit 598c1f7.
|
Since this PR introduced a regression, I have reverted it for since Please file a new PR with a fixed implementation of this refactoring. |
|
@Mugen87 any ideas what this refactor should have broken? |
|
No, sorry. I did not have time to investigate this in detail. I can only say for sure that the |
|
I'll take a look asap. Thanks. |
|
🤦 after doing the refactor again this is the only difference. Will make a new PR now. 
|
Related issue:
Description
As it seems that more granular extension callbacks for animation are not desired, this PR merely refactors some of the animation code into separate methods, which can then be called from other code. The methods stay internal; I hope that satisfies the wish to not increase the to-be-maintained API surface.
This allows to have a separate file that only uses the existing
loadAnimationhook for KHR_animation_pointer, at the price of some code duplication in the extension compared to my earlier PR.See https://github.com/needle-tools/three.js/blob/khr-animation-pointer-separation/examples/jsm/loaders/GLTFLoaderAnimationPointer.js
cc @donmccurdy @takahirox
This contribution is funded by Needle